Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Bug in Academy: Not remembering scroll position. #822

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

snoattoh
Copy link
Contributor

@snoattoh snoattoh commented Jun 7, 2024

window.history.state was null thus the eventListener on scroll couldn't finish running, added push state after checking hash so that it should always have a value on load

@chasenlehara chasenlehara force-pushed the LD-203-history-state branch 4 times, most recently from 025a896 to 6c3a182 Compare August 16, 2024 01:12
This completely refactors how the header, sidebar, main content, and table of contents (TOC) are laid out.

Previously, JS was required to keep track of the scroll position on reload. This caused bugs (like the wrong position being restored on reload if you clicked a link in the TOC) and led to some features being lost (like the “tap to scroll to the top of the page” in Safari).

Now, the browser is able to keep track of the scroll position for the page, so no JS is required and the CSS layout is more robust.
@chasenlehara chasenlehara merged commit bcd2bba into main Aug 16, 2024
1 check passed
@chasenlehara chasenlehara deleted the LD-203-history-state branch August 16, 2024 14:38
chasenlehara added a commit that referenced this pull request Aug 16, 2024
This reverts commit bcd2bba.

Looks like there’s a bug when loading a URL with a fragment in it (the page does not automatically scroll down).
chasenlehara added a commit that referenced this pull request Aug 16, 2024
…839)

This reverts commit bcd2bba.

Looks like there’s a bug when loading a URL with a fragment in it (the page does not automatically scroll down).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants